Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flairs CSS animations #3027

Merged
merged 81 commits into from
Nov 14, 2024
Merged

Add flairs CSS animations #3027

merged 81 commits into from
Nov 14, 2024

Conversation

MonkeyDo
Copy link
Member

@MonkeyDo MonkeyDo commented Nov 7, 2024

Adding a new .less file with the CSS definitions for flair animations.
We'll want to review if we want to keep all of those, and think about others to add, but this should get us started nicely.

Add the .flair CSS class along with one of the animation CSS classes (.shake,.flip-3d, etc.).

Some of these effects require manipulating the text in one of two ways:

  1. easy one: adding the data-text="username" HTML attribute to the containing element , with the actual username of course
  2. spreading each letter in the username to a separate span HTML element. this allows animating each letter with a delay

MonkeyDo and others added 30 commits October 25, 2024 18:50
Youtube forces us to follow their branding guidelines to the letter, so we need to force a minimum height of 20px for the icon path inside the svg.
For the Brainzplayer UI I made all icons bigger, for the artist page just the youtube icon.
This will be done in part 2, in the meantime since this page is out, clarify that they are coming soon, as suggested by aerozol
Updated AddData.tsx with ListenBrainz macOS Scrobbler for Music.app
Donations: add a note that user flairs are coming soon
…crobbler-for-Music.app

Update AddData.tsx with ListenBrainz macOS Scrobbler for Music.app
After #2991 , Last.FM imports are managed in the back-end, making this front-end manual import a duplicated we want to get rid of.

Keep the mechanism as we still use it for Libre.FM imports, and rename the components accrodingly, making them work by default and primarily for LibreFM.
Once we have converted LibreFM import as well we can get rid of the front-end importer components
it was in the import page previously, but now that LasftFM has moved to the connect services page the import-love-tracks feature also needed to be moved to be available there.
Add some more logic to show the "see full discography" button only if there are more than 4 rows in total across type categories.
This can show up to 4 categories of one row, 2 categories of 2 rows, or  1 of 2 rows + 2 of one rows.
LB-1664: Only show "full discography" button when required
Copy link
Member

@anshg1214 anshg1214 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks great! Just some minor comments, and then we're ready to merge.

One thing, for all the animations, can we avoid using the link-color blue, and use the default text color instead?

frontend/js/src/settings/Settings.tsx Outdated Show resolved Hide resolved
frontend/js/src/settings/Settings.tsx Outdated Show resolved Hide resolved
frontend/js/src/settings/Settings.tsx Show resolved Hide resolved
frontend/css/flairs.less Outdated Show resolved Hide resolved
frontend/css/flairs.less Outdated Show resolved Hide resolved
<h3>Flair Settings</h3>
<p>
Choose which flair you want your username to show to let other
users know you donated. ListenBrainz.
Unlocked by <Link to="/donate/">donating</Link>. Some flairs are
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add a modal here explaining how do we calculate the time duration for which the flairs remain?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this is crucial and currently missing, I had the same thought.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I have, would love to hear @Aerozol's feedback as well:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was important too, glad to see you're already onto it!

My usual text changes, take or leave as you please:
"Every $5 donated unlocks flair for 1 month. Larger donations will unlock multiple months of flair. Multiple donations will stack."
(I assume this is all correct now, after lucifer changed the donation calculation system?)

Two additional points (sorry!)

  1. We will have to mention that the flair isn't forever on the donations page as well - we can also add the same modal/modal text.
  2. Since it's getting a bit confusing we should probably show how many months of flair are left/have been built up :S This seems like the right place for it. e.g. "You have x days of flair remaining". (display nothing if they have not donated)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggested improvements!
I reworked the text a bit myself along the same lines, and added the same tooltip on the donate page.

Point # 2 about adding the unlock time left for a user is definitely a good idea, but I think probably best for a follow-up PR as I already stretched the scope of this one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I messed up and pushed commits to the feature branch directly instead of opening PR...

But here it is, using the existing nag-check endpoint to get the remaining days of unlock, or warn the user otherwise:
image

image

Tooltip is the same message as just above:
image

anshg1214 and others added 4 commits November 13, 2024 17:30
LB-1660: Sort by listen count on artist pages doesn't properly sort
Use link color only for anchor elements (default to regular text color), make inner span elements inline for display (otherwise they can wrap), and adjust the sliced effect position.
Copy link
Member

@anshg1214 anshg1214 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR Looks really good now but some flairs are broken now.

The Flair Options Wave, Flip Horizontal, Flip Vertical, Flip 3D, and Tornado are broken, and I cannot see any animation.

It's probably after you tweeked some css as I can see here.

@MonkeyDo
Copy link
Member Author

The PR Looks really good now but some flairs are broken now.

The Flair Options Wave, Flip Horizontal, Flip Vertical, Flip 3D, and Tornado are broken, and I cannot see any animation.

It's probably after you tweeked some css as I can see here.

Yes, I realized too, thanks !
Looking into it, but lunch break first

+ also uses the Username component at the top of the page
MonkeyDo and others added 2 commits November 13, 2024 17:12
Remove deprecated manual LastFM importer
also adds the same explanation tooltip to the donate page
@pep8speaks
Copy link

Hello @MonkeyDo! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 12:131: E501 line too long (138 > 130 characters)

Line 76:5: E303 too many blank lines (2)
Line 100:5: E303 too many blank lines (2)

Copy link
Member

@anshg1214 anshg1214 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@anshg1214 anshg1214 merged commit 6720d6d into donation-flairs Nov 14, 2024
4 checks passed
@anshg1214 anshg1214 deleted the flairs-css branch November 14, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants